-
-
Notifications
You must be signed in to change notification settings - Fork 368
feat: Deeplinking for the forgot password page #5362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
feat: Deeplinking for the forgot password page #5362
Conversation
You have a formatting issue on your code: https://github.com/openfoodfacts/smooth-app/actions/runs/9489564735/job/26151127165?pr=5362 Running |
Sorry, I missed this. I will fix this |
It seems your commit partially fixes the issue. ![]() |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5362 +/- ##
==========================================
- Coverage 9.54% 5.89% -3.66%
==========================================
Files 325 493 +168
Lines 16411 29458 +13047
==========================================
+ Hits 1567 1737 +170
- Misses 14844 27721 +12877 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Thanks for the fix. |
Let me check |
[ Screen.Recording.2024-07-06.at.1.39.36.PM.mov](url) It goes back to the home page |
@@ -282,6 +287,8 @@ class _SmoothGoRouter { | |||
} | |||
} else if (path == _ExternalRoutes.MOBILE_APP_DOWNLOAD) { | |||
return AppRoutes.HOME; | |||
} else if (path == _ExternalRoutes.FORGOT_PASSWORD) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move this statement before or after SIGNUP
?
It will allow the regrouping of user management routes.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay will do
Heyy, there are some conflicts, but they seem to be just a few lines. |
Yeah ill have this cleaned up this week really by tomorrow |
@jnnabugwu are you still working on this? :) |
@g123k This is ready to be merged are there any other changes needed? |
@jnnabugwu this PR does not pass the required checks yet. Take a look why:
|
Will have this done by next week |
@jnnabugwu should we close this PR? Or are you still working on it? |
let me do this now! |
@g123k Can you look at this pr again sorry for the long wait and thanks for the patience. |
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces deep linking for the forgot password page, enhancing the user experience by allowing direct access to the password reset functionality from external links. The changes appear well-structured and integrate smoothly with the existing navigation system. However, there are a few points that could be improved to enhance code clarity and maintainability.
Summary of Findings
- Route Definition Consistency: The naming and structure of internal routes should be consistent across the application to improve maintainability and reduce potential errors. Ensure that the naming convention for routes like
FORGOT_PASSWORD_PAGE
aligns with other similar routes. - External Route Handling: When adding new external routes, it's important to consider all potential use cases and ensure that the application handles them gracefully. Verify that the new
FORGOT_PASSWORD
external route is correctly processed and redirects users as expected. - Code Comment Completeness: Ensure that all code additions, especially those related to routing and navigation, are adequately commented to explain their purpose and functionality. This helps in maintaining code clarity and facilitates future modifications.
Merge Readiness
The pull request is almost ready for merging. Addressing the identified issues related to route definition consistency, external route handling, and code comment completeness will further enhance the quality and maintainability of the code. I am unable to approve this pull request, and recommend that another reviewer approves this code before merging.
What
Added deep linking for the forgot password page.
can be tested at https://world.openfoodfacts.org/forgot-password/
Screenshot
Screen.Recording.2024-06-12.at.4.41.26.PM.mov
Fixes bug(s)
Part of